-
Notifications
You must be signed in to change notification settings - Fork 972
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix various encoding problems with config 10895 #10900
base: master
Are you sure you want to change the base?
Fix various encoding problems with config 10895 #10900
Conversation
Sorry there is a problem to run tests and no much time to figure out the problem. |
The good news: all the (non-Windows) build regression tests pass with this change (annoyingly, it's a bit difficult right now to make these visible to contributors) The bad news: While the intention was always to be UTF-8 only, as I mentioned at #10896 (comment) this is likely a breaking (backward-incompatible) change for those on Windows who might already have characters in their configs which have different representation in their system charset vs UTF-8. Will need to think about whether this is sensible or not, and take me a while to review and add-back appropriate tests to bring confidence there aren't further regressions here. |
May be need enhanced migration? And may be bump version after which config will use utf only? |
Can you be specific about what you mean and what happened? The behaviour may depend on your system's default encoding and which character you are trying to use. What is the default encoding that your JVM is using on Windows? Also, did you try overriding the encoding per my comment at #10895 (comment) to confirm that works OK? |
This pull request has been automatically marked as stale because it has not had activity in the last 14 days. It will be closed in 7 days if no further activity occurs. |
Hi @cinex-ru - I've done seem testing myself and the behaviour is indeed strange. For me with the system default as "System properties": {
"file.encoding": "Cp1252",
... I can save non-ascii into a task command without an error being displayed and initially it gets persisted to config.xml garbled, but when I restart the server somehow it gets 'fixed' and repersisted as UTF-8 correctly. I don't get an error when, say editing task commands, but do via the admin config editor, as you describe. Can you tell me what the system This will help me understand the various setups people might be in and whether changing this might might things worse for them, or whether I need to (and can) do some fix-up migration. |
Issue: #10895
Description:
Fix problems with cruise-config encoding when saving from admin panel and from task and when migration performed.